in_syslog: initialize parser timestamps for stream records - #12447
Conversation
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe syslog parser now zero-initializes its output timestamp before parsing. A new integration test starts Fluent Bit, sends timestamp-free JSON messages through syslog input, and verifies both messages are processed while the daemon remains running. ChangesSyslog timestamp handling
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5e06c4168
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| address = str(tmp_path / "input.sock") if plugin == "unix_socket" else ("127.0.0.1", port) | ||
| parser = tmp_path / "parsers.conf" | ||
| parser.write_text("[PARSER]\n Name json\n Format json\n") | ||
| command = [os.environ["FLUENT_BIT_BINARY"], "-f", "0.1", "-R", str(parser), "-i", plugin] |
There was a problem hiding this comment.
Honor the suite's default binary lookup
When this scenario is run through the documented default ./run_tests.py flow without setting FLUENT_BIT_BINARY, this direct environment lookup raises KeyError before Fluent Bit starts, even when build/bin/fluent-bit exists. Use the suite's binary resolver/manager or provide the same default-path fallback used elsewhere.
AGENTS.md reference: AGENTS.md:L62-L65
Useful? React with 👍 / 👎.
| command += ["-o", "stdout", "-m", "*", "-p", "format=json_lines"] | ||
| log = tmp_path / "fluent-bit.log" | ||
| memlog = tmp_path / "valgrind.log" | ||
| memory = os.environ.get("VALGRIND") == "1" |
There was a problem hiding this comment.
Run the test under the selected macOS memory checker
On macOS, invoking the required memory-safety pass with LEAKS=1 LEAKS_STRICT=1 leaves memory false because this custom launcher only recognizes VALGRIND; the test consequently runs Fluent Bit directly and can report success without checking leaks. Handle the Leaks mode or use the shared process manager that supports both platform checkers.
AGENTS.md reference: AGENTS.md:L101-L105
Useful? React with 👍 / 👎.
Clear the output timestamp before parsing each stream record. Parsers without a configured time key can leave that value untouched, causing uninitialized reads and invalid record timestamps.
Validation: The baseline triggered Valgrind uninitialized-value errors. The JSON-parser regression passed normally and under strict Valgrind after the fix. Existing syslog TCP, TLS, UDP and Unix socket scenarios passed normally and under strict Valgrind.
Focused integration commands (run against the combined validation build containing these fixes):
Full PR-range commit-prefix lint, DCO and whitespace checks passed against fetched master. Code and regression tests are separate commits.
Compatibility: Records without a parsed timestamp use the existing current-time fallback. Parsed timestamps and configuration interfaces are unchanged.
Summary by CodeRabbit
Bug Fixes
Tests